Skip to content

build: add a cmake based build system #196

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 19, 2017
Merged

Conversation

compnerd
Copy link
Member

This is far from complete, but is sufficient to build a Linux version of
libdispatch. It shows what a potential cmake based build system could
look like, and if desired can be completed to build all the various
flavours with cmake.

@compnerd
Copy link
Member Author

CC @llvm-beanz

Lets get the party started!

@compnerd
Copy link
Member Author

compnerd commented Jan 3, 2017

Gentle post-holiday reminder ping :-)

@gottesmm
Copy link
Contributor

gottesmm commented Jan 4, 2017

@mwwa @dgrove-oss

Thoughts?

@mwwa
Copy link
Contributor

mwwa commented Jan 4, 2017

cc @das

@dgrove-oss
Copy link
Contributor

In principle, using cmake to build libdispatch would be a nice simplification (one less build system to play with). I personally don't know cmake that well, but am willing to learn...

We should wait for @das to give his opinion. I don't have the big picture of how this plays with the internal build systems for dispatch.

@compnerd
Copy link
Member Author

compnerd commented Jan 4, 2017

@dgrove-oss yeah, I think that it would be a pretty big simplification. libkqueue and pthread-workqueue are now CMake based as well, which are used for Linux and android for now.

Furthermore, switching to CMake would mean that cross-compiling libdispatch for windows would be much easier. With swift building and running on Windows (cross-compiled) once again, Id like to see dispatch building as well.

@MadCoder
Copy link
Contributor

MadCoder commented Jan 4, 2017

I don't think @das will be opposed to CMake, for all I care, the less we use autotools the better, but it has to be 100% backward compatible and generate the config.h header properly and all that. I currently have no time to review such a change.

internally we use an xcode project to build, so cmake/autotools make no difference whatsoever.

last, there's virtually no chance cmake will help significantly with windows, libdispatch requires porting first, the build system is a tiny problem compared to that.

@compnerd
Copy link
Member Author

compnerd commented Jan 4, 2017

@MadCoder actually, since Ive been doing most of the work Linux, getting it to the point where I can build to actually look into the porting issues would be significantly helpful :-). Or is there a simple way to build with clang-cl that I overlooked?

Yes, cmake can generate the config.h header properly, with detection and everything.

@gottesmm
Copy link
Contributor

@das ping!

@gottesmm
Copy link
Contributor

@MadCoder @mwwa Can one of you ping @das a bit harder on this? I have tried to ping him here and via email and have failed.

@das
Copy link
Contributor

das commented Feb 13, 2017

@gottesmm sorry, this fell between the cracks, will reply in detail asap

@gottesmm
Copy link
Contributor

@das Thanks! I appreciate it = ).

@compnerd
Copy link
Member Author

Since @gottesmm was asking about building with this: this seems to be sufficient to build libdispatch on Linux for me with the latest changes (which enable a couple of options).

cmake -G Ninja /Users/compnerd/Source/apple-swift/swift-corelibs-dispatch -DCMAKE_C_COMPILER=/usr/x86_64-pc-linux-gnu/bin/clang -DCMAKE_CXX_COMPILER=/usr/x86_64-pc-linux-gnu/bin/clang++ -DWITH_KQUEUE=/Users/compnerd/Source/libkqueue -DWITH_BLOCKS_RUNTIME=/Users/compnerd/Source/llvm/projects/compiler-rt/lib/BlocksRuntime

@dgrove-oss
Copy link
Contributor

does it support building and running the test suite?

@compnerd
Copy link
Member Author

@dgrove-oss unfortunately, not fully yet. Happy to have a bit of help :-). You should have a look at the last push, it has partial support for a subset of the tests. You should be able to add -DENABLE_TESTING=YES when configuring with cmake, and then run ninja test to run the tests (it uses CMake's ctest).

@compnerd
Copy link
Member Author

@dgrove-oss if you have kqueue, pthread-workqueues, and BlocksRuntime installed, then the test suite should build (and hopefully pass). This needs a bit more work to support building it as part of libdispatch still.

@compnerd
Copy link
Member Author

@dgrove-oss okay, so, the current version should permit building and running the test suite (though, Ive not had it fully pass yet). Im able to just run ninja test to run the test suite though. It also now supports building pthread-workqueues and libkqueue as subprojects. They require newer versions (as they have been converted to CMake).

@dgrove-oss
Copy link
Contributor

Thanks @compnerd. I believe there are still some Swift 3.1 changes in flight that might require us to merge libdispatch master to swift-3.1-branch. Once we are done dealing with 3.1, I think it makes sense to look at ditching autotools and switching to CMake on master for Swift 4 development.

@compnerd
Copy link
Member Author

@dgrove-oss that makes sense. However, I think that it would be invaluable to get some early feedback on the work and get a rough idea of what things to prioritize. Switching out the build system, especially one this complex is a slightly larger undertaking, and making sure that the pieces others care about are taken care of first can be helpful.

@compnerd
Copy link
Member Author

@das ping

@das
Copy link
Contributor

das commented Feb 21, 2017

apologies for the delay, got busy getting the changes ready that I just pushed to darwin/trunk branch. I agree with @dgrove-oss that we should only tackle this after that large merge has landed.

Switching to a build system that fits better in the swift ecosystem and has more potential maintainers in the community than the current one sounds like a very good step forward to me.

I would however like to avoid adding a 3rd parallel buildsystem to this project longer term (when we already don’t maintain the 2nd autotools-based one particularly well, e.g. it is still currently broken on Darwin, mainly because I'm the only one that maintains it for that platform and don't actually use that buildsystem myself day-to-day).

This means that a cmake based buildsystem would need to be able to replace the current supported functionality of the autotools buildsystem, if not immediately then at least it needs to to be possible to add that functionality over time.

In previous discussions about switching to cmake (during the initial move of the project from macosforge to github), the consensus was that would be a non-trivial undertaking that was significant enough to not enter into at the time.

If you are willing to commit to spearheading such a larger effort, I'd be happy with doing that via incremental changes over some amount of time (culminating in removal of the autotools buildsystem), e.g. with a target of having everything replaced for the Swift 4 deadline.

Initially I think we would at the minimum need to support the primary Linux distros used by swift, as well the current OSX release, along with building the testsuite for both of these as discussed above (in a way that can be used by the swift CI)

For OSX, there are two important scenarios:

  1. we need to be able to start building for Darwin in the swift CI, in order to qualify changes for that platform on an ongoing basis (and avoid breaking the buildsystem for Darwin...). At present I discover such breakage weeks after PRs are merged when I pull github changes back into our internal repository and we perform testing on Darwin.
    This requires a libdispatch.dylib that can override the system one via DYLD_LIBRARY_PATH (at least that is how the autotools make test does it currently, static linking into the tests may also work)

  2. we need to support building a drop-in replacement of the OS dylib that can be installed into /usr/lib/system (as described in INSTALL.md), so that contributors can test and experiment with changes on OSX on a whole-system basis. This means the new buildsystem would need to produce a bit-by-bit identical library to the dylib that the autotools buildsystem would produce (I can help with validating that).

I think 2. would be fine as a second step separate from this PR (it is something that I have historically only updated for the opensource drop associated to major OSX releases, which never happened this year due to the change in the opensourcing process of libdispatch).

The next step would be to support the additional platforms that the autotools buildsystem currently nominally supports. This would primarily be *BSDs and older OSX releases (back one major OSX release is our current policy although we have kept some older compatibility support so it should be possible be able to add building further back if that became necessary).
In particular the autotools buildsystem was originally contributed as part of the *BSD port, so I don't think it would be appropriate to remove that buildsystem without providing a replacement for those platforms.

I hope that makes sense? I'm happy to continue discussion of the initial step in this PR but it would probably be more appropriate to hash out details of how to organize the larger plan separately (e.g. in a github project ? or in a different PR or over email on the list ?)

CMakeLists.txt Outdated
set(CMAKE_LIBRARY_TYPE STATIC)
endif()

set(WITH_KQUEUE "" CACHE PATH "Path to kqueue headers")
Copy link

@hughbe hughbe Feb 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think KQUEUE has been removed in #215?

@compnerd
Copy link
Member Author

@das thanks for the reply.

I completely agree, I would hate to see a third build system come into the picture. Experiences with LLVM has shown that maintaining two build systems is painful enough. Yes, I believe that it should be possible to fully replace all the functionality of the autotools based build with the cmake based build.

Doing it incrementally would be wonderful. It is pretty complex to try to do it in a single shot. Plus, with the incremental approach, others could pitch in and help. I think that targeting the swift 4 release is a great idea.

I believe that the CMake system should accomodate building for the BSD targets as well as simplify the android build.

@compnerd
Copy link
Member Author

Okay, it almost works for Linux now. The inclusion of pthread-workqueues, which doesnt occur with the autotools based build, is what keeps this from being complete enough for building on Linux. If you tweak that, then it is able to build and pass all the tests on Linux.

@llvm-beanz
Copy link

@das, in my experience producing "bit-by-bit identical" outputs from CMake and autoconf build systems is incredibly challenging, and may not be reasonable to achieve in this situation.

In autoconf builds the compile and link commands are constructed from fairly simple string processing. CMake takes a very different approach to how it generates build steps, which makes it very difficult to perfectly match an autoconf-generated binary.

The biggest difference between CMake and autoconf that impacts the the layout of the final binaries is how CMake handles library dependencies for targets that are also in the CMake build. Take this as a simple example (I use '->' to signify a dependency):

libFoo.a -> libGoop.a
libBar.a -> libGoop.a
libBaz.dylib -> libFoo.a, libBar.a

In autoconf you might write the linker command as:
ld -lBar -lFoo -lGoop <objects> -o libBaz.dylib

On most Unix systems CMake would generate the linker line as:
ld -lBar -lGoop -lFoo -lGoop <objects> -o libBaz.dylib

CMake does it this way to attempt to generate a minimally correct binary. The first -lGoop specification results in only pulling in symbols from Goop used by Bar, and the second specification pulls in only the symbols used by Foo which are not already pulled in. In common cases this ordering behavior is unimportant, however in situations where multiple archives are providing the same symbols it can be very important.

Because of these differences it is very challenging to make CMake generate identical binaries to autoconf. Also, because of how linkers are order-dependent on all platforms, it should not be necessary to produce "bit-by-bit identical" outputs to qualify build systems. It is certainly an easy way to qualify a build system if such identical outputs are reasonably achievable, but I would ask that you not block adopting CMake on identical outputs because it may not be reasonable to achieve.

@MadCoder
Copy link
Contributor

MadCoder commented Mar 1, 2017

by identical binaries, @das doesn't mean byte-to-byte but "same linked .o's with same options" so that it will reliably build dylib's that you can install on Darwin without hosing your OS.

a broken libdispatch is a broken OS.

@compnerd
Copy link
Member Author

compnerd commented Mar 1, 2017

@MadCoder that seems like a much more reasonable approach.

On the Linux side, we can build and run tests with the current patch set. Im hoping that someone can verify that the android builds also work. It should be possible to build this for FreeBSD as well I believe.

For the Darwin side, the biggest hurdle is that I do not have a build that I can use to replicate into CMake.

@compnerd compnerd force-pushed the cmake branch 2 times, most recently from 65dc3fb to e4ab806 Compare March 6, 2017 21:58
@das
Copy link
Contributor

das commented Mar 6, 2017

I had meant bit-by-bit identical, but only on the assumption that that would be the easiest thing to validate, if we can accurate validate 'functionally identical', that is certainly sufficient.

one issue for Darwin will be that we need additional dependencies (c.f. the El Capitan instructions in Install.md, which is the last time that worked with autoconf).

If you can provide some help in how we would express such things in CMake (AFAIR the main things needed were "configure time" detection of header absence, adding their locations in source code downloads to include paths, along with creation of compatibility symlinks for naming/location mismatches), I should be able to make some progress on getting the Darwin build underway.

However given that the Darwin build doesn't work now, IMO this shouldn't hold up this pull request. If all the other builds are in good enough shape to replace autoconf with and we are confident we can resolve the Darwin build issue in a separate PR, we should pull the trigger on this independently from that work.

This is far from complete, but is sufficient to build a Linux version of
libdispatch.  It shows what a potential cmake based build system could
look like, and if desired can be completed to build all the various
flavours with cmake.
@compnerd
Copy link
Member Author

compnerd commented Mar 7, 2017

@das that sounds wonderful!

I can provide help in those checks if you like. Most of the headers that need checks are already in the current CMake build already! The options may be slightly different from their autoconf counterparts (-D... rather than --...), but most of the machinery should already be present. The biggest thing would be building and running the tests I think. I think that the biggest thing missing for the darwin builds atm is the symlink construction, but that should be relatively easy to accomplish as its a matter of just identifying what symlinks need to be constructed.

I've been testing on Linux, and it works fine. This should be just about ready for Android (there is one check that I believe needs tweaking -- for pthread). The other Android specific issues should be present without the CMake build. I believe that FreeBSD should also work with the current CMake based build. Once the pwq dependency is removed, I think that we can further simplify the build.

I think that addressing the Darwin builds in a separate PR is also a viable way forward. Getting this merged earlier means that others can start playing with this more easily as well and it would get a longer soak period.

@gottesmm
Copy link
Contributor

@das @compnerd So can we merge this?

@compnerd
Copy link
Member Author

Yeah, I think that was the consensus. I dont have push rights to this part of swift though.

@gottesmm
Copy link
Contributor

@das Do you mind if I pull the trigger here?

@das
Copy link
Contributor

das commented Apr 19, 2017

that is fine with me if everybody is happy about the state of this build system for Linux, @dgrove-oss ?

However I would like to see us switch over the CI linux build to this new buildsystem soon (and adjust documentation and the autoconf buildsystem to indicate the switchover for Linux), so that we don't have a buildsystem in the repo that we are not testing on an ongoing basis.

I would have preferred for that to happen in this PR but I'm ok with it being split out if that happens soon

@compnerd
Copy link
Member Author

Im happy to work on those items in follow up patches to get us quickly switched over (I think we run the risk of diverging build systems otherwise), but I think it will make it easier for others to actually start pitching in as well if this gets merged first.

@das
Copy link
Contributor

das commented Apr 19, 2017

sound good, thanks!

@das das merged commit 3da8398 into swiftlang:master Apr 19, 2017
@compnerd compnerd deleted the cmake branch April 19, 2017 21:14
das added a commit that referenced this pull request May 25, 2017
build: add a cmake based build system

Signed-off-by: Daniel A. Steffen <dsteffen@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants